-
Notifications
You must be signed in to change notification settings - Fork 128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(agent): add option to override security.openshift.io/v1 API presence #2051
base: main
Are you sure you want to change the base?
Conversation
Hi @jsecchiero. Thanks for your PR. After inspecting your changes someone with write access to this repo needs |
@@ -404,3 +404,6 @@ tests: | |||
tag: 1.31.2 | |||
# Allow to modify DNS policy | |||
dnsPolicy: null | |||
# Overrides `security.openshift.io/v1` API detection | |||
# useful while using "helm template" and to generate security context constraints | |||
hasAPISecurityOpenshiftV1Override: false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given (as discussed):
• If .Values.hasAPISecurityOpenshiftV1Override is provided the SCC template will use that value, overriding (.Capabilities.APIVersions.Has "security.openshift.io/v1")
• If .Values.hasAPISecurityOpenshiftV1Override is not provided it will use (.Capabilities.APIVersions.Has "security.openshift.io/v1")
I would be inclined not to provide a default value here. This means the default behaviour will be to use (.Capabilities.APIVersions.Has "security.openshift.io/v1")
and only if a value is purposefully provided will we override it.
I don't know if that has documentation implications though (ie will the chart docs not mention this value and essentially make it a secret override only found by reading charts/agent/templates/securitycontextconstraint.yaml
? Maybe that is a desired side-effect?
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days |
What this PR does / why we need it:
Checklist
feat(agent,node-analyzer,sysdig-deploy):
)